-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Apply the custom global styles CSS variables on the server #21420
Conversation
* @return string New block output. | ||
*/ | ||
function gutenberg_apply_style_attribute( $block_content, $block ) { | ||
if ( isset( $block['attrs'] ) && isset( $block['attrs']['style'] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally this doesn't rely on the attributes presence but on the "support flag". We need server-side awareness of support flags. cc @gziolo did you work on that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not yet, the plan was to come up with something better than supports
:)
Anyway, supports
works on the server and can be exposed when registered there to the client. So we should be good in that context. You just need to say we want it and we can implement everything. @aduth has some custom code that shims classname
or align
flags on the server. We can add the same logic for everything else.
Let me know whether we should update https://github.com/WordPress/gutenberg/blob/master/docs/rfc/block-registration.md#backward-compatibility and the part that states that supports
is client-side only and exists only for backward compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm thinking more and more that supports is actually a good thing if done right. Don't know what you'll think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably fine to keep it as is but make it more configurable. We still need to use data that can be serialized but we shouldn't limit ourselves to boolean
.
Size Change: -4 B (0%) Total Size: 889 kB
ℹ️ View Unchanged
|
|
||
// Try replacing an existing style | ||
$style_regex = '/((^\s*<[a-zA-Z]+\b[^><]*)style="([^"]*)")([^><]*>.*)/'; | ||
$updated_content = preg_replace_callback($style_regex, function($matches) use($extra_styles) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the preg_replace*
calls use $limit = 1
?
Hi @youknowriad, is this approach something we still want to follow in the short term or it is not something we should give priority for now? |
I think it's something to keep at hand but not something to pursue immediately. Let's focus on adding more editor features support and global styles support to theme.json and blocks for now. |
This PR tries to move the CSS styles variables generation to the server to avoid persisting these in the markup.
Some thoughts so far: